-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: update compression to be more aggressive #2962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded optional compression options to the in-browser image compression utility and updated the chat input to call it with specific constraints (size, dimensions, quality). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CI as ChatInput (Web)
participant IU as Image Utility
U->>CI: Drop/Paste image file
CI->>IU: compressImageInBrowser(file, {maxSizeMB:0.2, maxWidthOrHeight:512, quality:0.6})
Note right of IU: Merge options with defaults<br/>Perform compression
IU-->>CI: Compressed image (Blob/File) or error
CI-->>U: Proceed with upload/send or show error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
maxSizeMB: compressionOptions?.maxSizeMB ?? 0.2, | ||
maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 512, | ||
quality: compressionOptions?.quality ?? 0.6, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a discrepancy between the PR description and implementation. The PR description states that the default compression values should be:
maxSizeMB: 2
maxWidthOrHeight: 2048
quality: 0.8
However, the implementation sets more aggressive defaults:
maxSizeMB: 0.2
maxWidthOrHeight: 512
quality: 0.6
These more aggressive defaults will result in smaller file sizes but potentially lower image quality than what was specified in the PR description. Consider aligning the implementation with the documented defaults to ensure consistent behavior.
maxSizeMB: compressionOptions?.maxSizeMB ?? 0.2, | |
maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 512, | |
quality: compressionOptions?.quality ?? 0.6, | |
maxSizeMB: compressionOptions?.maxSizeMB ?? 2, | |
maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 2048, | |
quality: compressionOptions?.quality ?? 0.8, |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
compressImageInBrowser
inimage.ts
updated for more aggressive image compression with new default settings and optional configuration.compressImageInBrowser
inimage.ts
now uses more aggressive default compression options:maxSizeMB
reduced to 0.2,maxWidthOrHeight
reduced to 512, andquality
set to 0.6.compressionOptions
parameter tocompressImageInBrowser
for flexible tuning of compression settings.This description was created by
for cbc3f98. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit